refactor: slim DeviceSpec to required fields + sparse serialization#3170
refactor: slim DeviceSpec to required fields + sparse serialization#3170
Conversation
f477763 to
9a7aaf7
Compare
|
Does the CLI still populate |
@pedro18x no, the CLI doesn't use the deviceSpec payload to the API yet. |
I see. Approved! |
…lts, and computed properties Required: model, os. Optional with defaults: locale (narrowed per subtype), orientation, disableAnimations, cpuArchitecture, snapshotKeyHonorModalViews. Derived values (osVersion, deviceName, tag, emulatorImage) are now computed get() properties rather than stored fields. Deletes DeviceSpecRequest and DeviceSpec.fromRequest(); callers will be updated in follow-up commits.
Avoids running fromString() validation on every default construction.
AndroidLocale is a data class (not an enum) so its default remains
AndroidLocale.fromString("en_US") for now.
…irectly Removes DeviceSpecRequest usage and explicitly passes model/os defaults at each construction site. Uses ifBlank fallbacks for cases where AvdInfo or simctl parsing may produce empty strings.
…pec directly Threads CLI-provided device options into platform-specific DeviceSpec constructors with explicit fallbacks for missing model/os/locale values.
…tent in StartDeviceCommand
Remove stale DeviceSpecRequest import from CloudInteractor and replace the DeviceSpec.fromRequest(DeviceSpecRequest.Android()) demo call in AnsiResultView.main() with a direct DeviceSpec.Android(model, os) construction.
Smart-cast deviceSpec to DeviceSpec.Ios in AppValidator.validateDeviceCompatibility before accessing osVersion, which is now a subtype-local property.
…ializers Custom JsonSerializer<DeviceSpec> reads primary constructor defaults via Kotlin reflection and omits fields whose runtime value matches the default. Produces wire format containing only platform, model, os, and any fields that deviate from defaults. Adds per-subtype locale deserializers (AndroidLocale, IosLocale, WebLocale) because the narrowed locale field types on DeviceSpec subclasses cannot accept the base DeviceLocale interface returned by the legacy deserializer.
The sparse serializer iterates only primary-constructor parameters, so computed get() properties are already excluded regardless of annotation. Removes serialization coupling from the data class.
Replaces scattered hardcoded model/os strings at call sites with a single named DEFAULT constant per subtype. Parse-or-default sites branch to the full default when input is incomplete.
Removes disableAnimations, snapshotKeyHonorModalViews, and orientation from DeviceSpec subtypes. These are driver-level concerns sourced from WorkspaceConfig, not provisioning identity. Copilot-side worker already reads these values from WorkspaceConfig (see copilot#2398).
…sealed class Lets callers access these on a base DeviceSpec without per-subtype casts. Required by the copilot backend's response builders that work with a DeviceSpec without knowing its concrete subtype.
9a7aaf7 to
b9c537f
Compare
Problem
DeviceSpecrequired callers to provide values that have permanent, never-changing defaults (locale, cpuArchitecture, etc.) and persisted a handful of fields that are pure derivations (osVersion,deviceName,tag,emulatorImage). The result: verbose, noisy payloads, lots of unnecessary stored values in the DB, and confusion about which fields actually carry intent.It also mixed two different concerns: provisioning identity (what device to boot) and driver-level runtime config (how to configure the driver after boot). Driver-level config already lives in
WorkspaceConfig, where the CLI has always read it from; only the cloud worker was pulling it fromDeviceSpec.Separately,
DeviceSpecRequestwas a parallel nullable-everything hierarchy that existed only sofromRequest()could magically fill in defaults — indirection without a clear reason to exist now that Kotlin constructor defaults do the same thing.Approach
DeviceSpecbecomes provisioning identity only:platform,model,os,locale, pluscpuArchitectureon Android.disableAnimations,snapshotKeyHonorModalViews,orientation) are removed — consumers read them fromWorkspaceConfig.platforminstead.platform + model + osfrom callers. Everything else has a sensible default in the primary constructor.osVersion,deviceName,tag,emulatorImage) become computedget()properties — not stored, not serialized.DEFAULTconstant per subtype (DeviceSpec.Android.DEFAULT, etc.) for call sites that just want a reasonable device.Final shape
Key changes
DeviceSpec.kt: Reshaped to provisioning-only.DeviceSpecRequestandDeviceSpec.fromRequest()deleted. Driver fields removed.DeviceSpecSparseSerializer.kt(new): customJsonSerializer<DeviceSpec>that reads primary constructor defaults via reflection (cached per subtype) and omits fields matching defaults.DeviceSpec.X.DEFAULTinstead of scattered hardcoded model/os strings.Wire format
Android with only required fields:
{"platform":"ANDROID","model":"pixel_6","os":"android-33"}With a non-default cpuArchitecture:
{"platform":"ANDROID","model":"pixel_6","os":"android-33","cpuArchitecture":"X86_64"}Coordination with copilot
This PR deliberately removes driver fields that the copilot worker currently reads. The rollout is phased:
disableAnimations/snapshotKeyHonorModalViewsfromWorkspaceConfig. Deploy first.Jackson
FAIL_ON_UNKNOWN_PROPERTIES = falseon the backend and worker mappers ensures existing DB rows with the now-removed fields continue to deserialize cleanly.Test plan